Skip to content

Custom Providers#3967

Closed
DOsinga wants to merge 11 commits into
mainfrom
fix/compatible-providers-import
Closed

Custom Providers#3967
DOsinga wants to merge 11 commits into
mainfrom
fix/compatible-providers-import

Conversation

@DOsinga

@DOsinga DOsinga commented Aug 8, 2025

Copy link
Copy Markdown
Collaborator

fixing: #3824

Developerayo and others added 11 commits August 4, 2025 13:21
Move hardcoded prompts from router_tool_selector.rs and permission_judge.rs
into reusable .md template files using the MiniJinja template system.

Changes:
- Add router_tool_selector.md template with {{tools}} and {{query}} variables
- Add permission_judge.md template for read-only operation detection
- Update router_tool_selector.rs to use render_global_file()
- Update permission_judge.rs to use render_global_file() with fallback
- Add proper error handling for template rendering failures
- All existing tests continue to pass

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
.placeholder("https://api.example.com/v1/messages")
.validate(|input: &String| {
if !input.starts_with("http://") && !input.starts_with("https://") {
Err("Inputed URL must start with either http:// or https://")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Err("Inputed URL must start with either http:// or https://")
Err("URL must start with either http:// or https://")

verify_secret_key(&headers, &state)?;

let key = name_to_key(&name);
let key = goose::config::extensions::name_to_key(&name);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the import change?

.clone()
.unwrap_or_else(|| {
format!(
"Custom {} provider",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Custom {} provider",
"{} (custom)",

.to_uppercase()
.replace(" ", "_")
.replace("-", "_")
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some duplication between here and the CLI, maybe that could be reduced?

/// custom providers
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum ProviderEngine {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we call this "format" elsewhere

.map_err(|e| anyhow::anyhow!("Failed to parse {}: {}", path.display(), e))?;

configs.push(config);
} else {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty else block

}
}

/// load custom providers

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// load custom providers

}

/// load custom providers
pub fn load_custom_providers(dir: &Path) -> Result<Vec<CustomProviderConfig>> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also duplicates a bit I think from routes/config_management

REGISTRY.read().unwrap().all_metadata()
}

pub fn refresh_custom_providers() -> Result<()> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any path here that actually returns an error?

@jamadeo

jamadeo commented Aug 14, 2025

Copy link
Copy Markdown
Member

closing in favor of #3824

@jamadeo jamadeo closed this Aug 14, 2025
@DOsinga
DOsinga deleted the fix/compatible-providers-import branch May 14, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants